feat(virtq): add high-level packed virtqueue producer/consumer api#1514
Open
andreiltd wants to merge 1 commit into
Open
feat(virtq): add high-level packed virtqueue producer/consumer api#1514andreiltd wants to merge 1 commit into
andreiltd wants to merge 1 commit into
Conversation
andreiltd
requested review from
danbugs,
dblnz,
devigned,
jprendes,
jsturtevant,
ludfjig,
simongdavies,
squillace and
syntactically
as code owners
andreiltd
force-pushed
the
tandr/virtq-api
branch
from
650cf31 to
c6ddbad
Compare
andreiltd
added
the
kind/enhancement
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces a higher-level packed virtqueue API in hyperlight_common::virtq that sits on top of the existing packed-ring primitives, adding buffer management, message framing helpers, concurrency validation (loom), and performance benchmarks.
Changes:
- Add high-level producer/consumer APIs (
VirtqProducer/VirtqConsumer) that manage descriptor-chain lifecycle and notifications. - Add buffer allocation abstractions (
BufferProvider) plus concrete allocators (BufferPool,RecyclePool) and zero-copy segment types. - Add a small wire header (
VirtqMsgHeader), loom-based concurrency tests, and Criterion benchmarks for the new APIs/pools.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| src/hyperlight_common/src/virtq/ring.rs | Exposes mutable access to readable elements to support high-level send-chain writing. |
| src/hyperlight_common/src/virtq/producer.rs | Implements VirtqProducer, chain builder/write APIs, submission/batching, and used-chain reclamation. |
| src/hyperlight_common/src/virtq/consumer.rs | Implements VirtqConsumer, receive+reply types, payload copying, and completion/notification logic. |
| src/hyperlight_common/src/virtq/buffer.rs | Defines BufferProvider, allocator errors/types, Segments, and zero-copy Bytes ownership glue. |
| src/hyperlight_common/src/virtq/pool.rs | Adds BufferPool and RecyclePool implementations for virtqueue buffer allocation. |
| src/hyperlight_common/src/virtq/msg.rs | Adds an 8-byte message header for kind/flags/correlation/payload sizing. |
| src/hyperlight_common/src/virtq/mod.rs | Re-exports new APIs, adds docs/quick-start examples, and defines shared error/notification types. |
| src/hyperlight_common/src/virtq/concurrency.rs | Adds loom-based interleaving tests with a shadow-atomic memory model. |
| src/hyperlight_common/Cargo.toml | Adds dependencies/dev-deps for the new virtq APIs, pools, loom tests, and benches. |
| src/hyperlight_common/benches/buffer_pool.rs | Criterion benchmarks for allocation/free patterns in the pools. |
| src/hyperlight_common/benches/virtq_api.rs | Criterion benchmarks for high-level virtq roundtrips under different allocator strategies. |
| src/hyperlight_common/benches/common/mod.rs | Shared benchmark harness (in-memory MemOps, pair construction, roundtrip drivers). |
| Justfile | Adds a test-loom target to run loom concurrency tests. |
| Cargo.toml | Adds unexpected_cfgs lint config for cfg(loom). |
| Cargo.lock | Locks new crate dependencies introduced by the virtq/pool/bench work. |
| .github/workflows/dep_code_checks.yml | Runs loom concurrency tests in CI. |
andreiltd
force-pushed
the
tandr/virtq-api
branch
3 times, most recently
from
11a9c24 to
5b74480
Compare
ludfjig
previously approved these changes
Jun 9, 2026
Comment on lines
+84
to
+85
| drv: core::cell::UnsafeCell<EventSuppression>, | ||
| dev: core::cell::UnsafeCell<EventSuppression>, |
Contributor
There was a problem hiding this comment.
any point in making these loom::cell::UnsafeCell ?
Member
Author
There was a problem hiding this comment.
I switched this to model the descriptor and event-suppression memory with loom UnsafeCell too. An interesting outcome of that switch was that loom started reporting data races when reading Descriptors and EventSuppressions.
The cause was our single stage read api, we read the whole struct in one call: acquire-loading the flags and then reading the body. If the flags indicated the entry was ours, the body had already been read and if not, we discarded the body and kept waiting for valid flags. That's sound as long as the discarded body is never used, but loom (correctly) saw the unconditional body read racing the concurrent write to the same cell.
So I split the read api for both Descriptor and EventSuppression into two stages: read_flags_acquire and read_body. It's a bit more verbose, but it keeps loom happy and makes the concurrency tests more meaningful.
Good catch!
andreiltd
force-pushed
the
tandr/virtq-api
branch
2 times, most recently
from
5b6f495 to
504679c
Compare
This patch adds a higher level api on top of the packed ring primitives in hyperlight_common so callers don't manage raw descriptors directly. - Add VirtqProducer/VirtqConsumer in producer.rs and consumer.rs that handle buffer allocation, chain lifecycle, and event suppression/notification decisions; expand virtq/mod.rs with the public API, builders, and docs. - Add buffer management: the BufferProvider trait and shared types in buffer.rs plus two concrete allocators - a two-tier variable-size BufferPool and a fixed-slot RecyclePool. - Add an 8-byte wire message header in msg.rs for message type discrimination and request/response correlation. - Add loom-based concurrency tests in concurrency.rs using shadow atomics - Add criterion benchmarks for the buffer pool and virtq API. Signed-off-by: Tomasz Andrzejak <andreiltd@gmail.com>
andreiltd
force-pushed
the
tandr/virtq-api
branch
from
504679c to
967e455
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This patch adds a higher level api on top of the packed ring primitives in hyperlight_common so callers don't manage raw descriptors directly.